-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Go: Rename "named type" to "defined type" #18489
Conversation
228de09
to
b5b8857
Compare
b5b8857
to
9d8ee08
Compare
9d8ee08
to
6aa57e1
Compare
6aa57e1
to
f9a87cd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Overview
This pull request refactors terminology by replacing references to "named type" with "defined type" throughout the codebase and documentation. Key changes include updated change notes, modified error messages, updated comment texts and labels in type extraction logic, and corresponding updates in database schema definitions.
Changes
File | Description |
---|---|
go/ql/lib/change-notes/2025-02-12-deprecate-namedtype.md | Updates change notes to deprecate NamedType in favor of DefinedType |
go/extractor/extractor.go | Updates comments, error messages, and logic to use DefinedType instead of NamedType |
go/extractor/dbscheme/tables.go | Renames tables and updates comments from NamedType to DefinedType |
go/extractor/trap/labels.go | Updates variable names and function logic to reflect DefinedType |
Copilot reviewed 34 out of 34 changed files in this pull request and generated no comments.
Tip: Copilot code review supports C#, Go, Java, JavaScript, Markdown, Python, Ruby and TypeScript, with more languages coming soon. Learn more
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Broadly looks good. I only have a couple of comments.
I initially thought that maybe cases where we change which class another extends
from (e.g. ComparableType
) might be a breaking change, but I think you avoided that by keeping NamedType
around as an alias for DefinedType
. Just noting that here for reference.
go/downgrades/b3da71c3ac204b557c86e9d9c26012360bdbdccb/upgrade.properties
Outdated
Show resolved
Hide resolved
go/ql/lib/upgrades/4bd57e093275e5e892dfb16b55ed4bd76ea662be/upgrade.properties
Outdated
Show resolved
Hide resolved
dc4e11c
to
1a52398
Compare
@mbg Thanks for catching the unedited upgrade scripts - I had to regenerate them and forgot to fix them. I've updated the comment too. I also ran DCA, just in case I messed something up when updating the stats file, and it showed no significant variation in anything. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All looks good to me now!
No description provided.